Skip to content

fix: set DSG_APP_ID to prevent self-identification deadlock#321

Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
mhduiy:config
Jan 22, 2026
Merged

fix: set DSG_APP_ID to prevent self-identification deadlock#321
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
mhduiy:config

Conversation

@mhduiy
Copy link
Contributor

@mhduiy mhduiy commented Jan 20, 2026

如果不手动设置APPID,dtk的dsgapplication会通过am的接口去获取appid,如果使用dconfig的程序是am,则是自己阻塞调用自己,会卡死

Set the DSG_APP_ID environment variable to ApplicationManagerConfig before initializing the application. This prevents the application manager from calling its own Identify interface when accessing its own dconfig, which would cause a deadlock.

Log: Fixed a potential deadlock issue when the application manager accesses its own configuration

Influence:

  1. Test application manager startup and initialization
  2. Verify that the application manager can read its own dconfig without hanging
  3. Test interaction with other DSG applications to ensure environment variable doesn't interfere
  4. Monitor system logs for any identification-related errors during startup

fix: 设置DSG_APP_ID环境变量防止自身识别死锁

在应用程序初始化之前设置DSG_APP_ID环境变量为ApplicationManagerConfig。这 可以防止应用程序管理器在访问自身dconfig时调用自己的Identify接口,从而避
免死锁发生。

Log: 修复了应用程序管理器访问自身配置时可能出现的死锁问题

Influence:

  1. 测试应用程序管理器的启动和初始化过程
  2. 验证应用程序管理器能够正常读取自身dconfig而不出现挂起
  3. 测试与其他DSG应用程序的交互,确保环境变量设置不会产生干扰
  4. 监控系统日志,检查启动过程中是否有识别相关的错误

PMS: BUG-345379

Summary by Sourcery

Bug Fixes:

  • Prevent a potential deadlock caused by the application manager invoking its own Identify interface while reading its own configuration.

@sourcery-ai
Copy link

sourcery-ai bot commented Jan 20, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Sets the DSG_APP_ID environment variable to ApplicationManagerConfig at application startup to prevent the application manager from self-identifying via D-Bus when reading its own configuration, which previously could cause a deadlock.

Sequence diagram for application manager startup with DSG_APP_ID set

sequenceDiagram
    participant System
    participant ApplicationManager
    participant DConfigService
    participant IdentifyInterface

    System->>ApplicationManager: start(argc, argv)
    ApplicationManager->>ApplicationManager: qputenv(DSG_APP_ID, ApplicationManagerConfig)
    ApplicationManager->>DConfigService: loadOwnDConfig()
    DConfigService->>IdentifyInterface: identify(DSG_APP_ID)
    IdentifyInterface-->>DConfigService: return identity for ApplicationManagerConfig
    DConfigService-->>ApplicationManager: return configuration
    ApplicationManager->>ApplicationManager: continue initialization without deadlock
Loading

Flow diagram for DSG_APP_ID preventing self-identification deadlock

flowchart TD
    A["ApplicationManager main() start"] --> B["Set environment DSG_APP_ID = ApplicationManagerConfig via qputenv"]
    B --> C["Access own dconfig via DConfigService"]
    C --> D["DConfigService calls IdentifyInterface with DSG_APP_ID"]
    D --> E["IdentifyInterface resolves to ApplicationManagerConfig without reentering ApplicationManager"]
    E --> F["Configuration returned successfully"]
    F --> G["ApplicationManager initialization completes without deadlock"]
Loading

File-Level Changes

Change Details Files
Initialize DSG_APP_ID before application startup to avoid self-identification deadlock in the application manager.
  • Call qputenv to set the DSG_APP_ID environment variable to ApplicationManagerConfig at the very beginning of main()
  • Ensure the environment is configured before any profiling, initialization, or D-Bus usage occurs so that the application manager does not call its own Identify interface when accessing its own dconfig
apps/dde-application-manager/src/main.cpp

Possibly linked issues

  • #(unknown): The PR implements support for the DSG_APP_ID environment variable as requested in the issue, preventing self-deadlock.

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 1 issue, and left some high level feedback:

  • Consider checking whether DSG_APP_ID is already set before calling qputenv so that the application manager does not unintentionally override a value provided by the environment or other components.
  • It would be helpful to add a brief code comment near the qputenv call explaining that this prevents the application manager from calling its own Identify interface and causing a deadlock, to make the rationale clear to future maintainers.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Consider checking whether DSG_APP_ID is already set before calling qputenv so that the application manager does not unintentionally override a value provided by the environment or other components.
- It would be helpful to add a brief code comment near the qputenv call explaining that this prevents the application manager from calling its own Identify interface and causing a deadlock, to make the rationale clear to future maintainers.

## Individual Comments

### Comment 1
<location> `apps/dde-application-manager/src/main.cpp:40` </location>
<code_context>

 int main(int argc, char *argv[])
 {
+    qputenv("DSG_APP_ID", ApplicationManagerConfig);
 #ifdef PROFILING_MODE
     auto start = std::chrono::high_resolution_clock::now();
</code_context>

<issue_to_address>
**suggestion (bug_risk):** Consider preserving a pre-set DSG_APP_ID instead of unconditionally overriding it

This unconditionally overrides any `DSG_APP_ID` already set in the environment, which could break setups that rely on their own value (e.g., debugging or multi-instance configurations). To treat this as a default instead, consider using `qEnvironmentVariableIsSet` / `qEnvironmentVariableIsEmpty` and only calling `qputenv` when the variable is not already defined.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

BLumia
BLumia previously approved these changes Jan 20, 2026
@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: BLumia, mhduiy

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Set the DSG_APP_ID environment variable to ApplicationManagerConfig
before initializing the application. This prevents the application
manager from calling its own Identify interface when accessing its own
dconfig, which would cause a deadlock.

Log: Fixed a potential deadlock issue when the application manager
accesses its own configuration

Influence:
1. Test application manager startup and initialization
2. Verify that the application manager can read its own dconfig without
hanging
3. Test interaction with other DSG applications to ensure environment
variable doesn't interfere
4. Monitor system logs for any identification-related errors during
startup

fix: 设置DSG_APP_ID环境变量防止自身识别死锁

在应用程序初始化之前设置DSG_APP_ID环境变量为ApplicationManagerConfig。这
可以防止应用程序管理器在访问自身dconfig时调用自己的Identify接口,从而避
免死锁发生。

Log: 修复了应用程序管理器访问自身配置时可能出现的死锁问题

Influence:
1. 测试应用程序管理器的启动和初始化过程
2. 验证应用程序管理器能够正常读取自身dconfig而不出现挂起
3. 测试与其他DSG应用程序的交互,确保环境变量设置不会产生干扰
4. 监控系统日志,检查启动过程中是否有识别相关的错误

PMS: BUG-345379
@mhduiy
Copy link
Contributor Author

mhduiy commented Jan 22, 2026

/forcemerge

@deepin-ci-robot
Copy link

deepin pr auto review

这段代码是在 main 函数开始时设置了一个名为 DSG_APP_ID 的环境变量。以下是对这段代码的详细审查和改进建议:

1. 语法逻辑审查

  • 语法正确性setenv 的函数签名为 int setenv(const char *name, const char *value, int overwrite);
    • name: "DSG_APP_ID" (正确)
    • value: ApplicationManagerConfig (假设这是一个宏定义或全局字符串常量,语法上没问题)
    • overwrite: 0 (表示如果环境变量已存在,则不覆盖)
    • 结论:语法是正确的。

2. 代码质量审查

  • 硬编码字符串"DSG_APP_ID" 是一个硬编码的字符串。如果这个字符串在项目中多处使用,建议将其定义为一个宏或常量,以便于维护和防止拼写错误。
  • 覆盖策略 (overwrite = 0):这里设置 overwrite 为 0,意味着如果 DSG_APP_ID 已经在环境中被设置(例如由父进程或启动脚本设置),程序将忽略这行代码并保留原有的值。
    • 潜在风险:如果该应用的运行严格依赖于 ApplicationManagerConfig 的值,而外部环境意外设置了一个错误的 DSG_APP_ID,程序可能会以错误的配置运行。
    • 建议:除非明确需要允许外部环境覆盖此配置,否则建议将 overwrite 设置为 1,以确保程序始终使用预期的配置 ID。

3. 代码性能审查

  • setenv 是一个系统调用,涉及到环境变量表的修改。虽然它有一定的开销,但通常只在程序启动初期调用一次,对整体性能影响可以忽略不计。
  • 结论:性能方面无需优化。

4. 代码安全审查

  • 环境变量注入:环境变量通常会被子进程继承。设置此变量可能会影响该程序后续启动的子进程行为。如果 ApplicationManagerConfig 包含敏感信息或路径,需确保其值是可信的。
  • 失败处理setenv 在失败时会返回 -1 并设置 errno(例如内存不足)。当前代码没有检查返回值。
    • 如果设置失败,程序后续逻辑可能会因为缺少该环境变量而崩溃或行为异常。
    • 建议:增加返回值检查,如果设置失败且该变量至关重要,应记录日志或进行错误处理。

5. 改进后的代码建议

int main(int argc, char *argv[])
{
    // 建议将字符串定义为常量,假设 ApplicationManagerConfig 是已定义的宏或字符串
    const char* const kDsgAppId = "DSG_APP_ID";
    
    // 设置 overwrite 为 1,强制使用代码中定义的配置 ID,避免外部环境干扰
    // 增加 error checking
    if (setenv(kDsgAppId, ApplicationManagerConfig, 1) != 0) {
        // 记录错误日志,例如使用 qWarning (假设这是 Qt 项目) 或 fprintf(stderr, ...)
        fprintf(stderr, "Failed to set environment variable %s: %s\n", 
                kDsgAppId, strerror(errno));
        // 根据业务逻辑,决定是继续运行还是退出
        // return -1; 
    }

#ifdef PROFILING_MODE
    auto start = std::chrono::high_resolution_clock::now();
#endif
    // ... 其他代码
}

总结

这段代码在功能上没有问题,但为了增强健壮性和可维护性,建议:

  1. overwrite 参数改为 1(除非业务逻辑允许外部覆盖)。
  2. 检查 setenv 的返回值并处理可能的错误。
  3. 将环境变量名称提取为常量。

@deepin-bot
Copy link

deepin-bot bot commented Jan 22, 2026

This pr force merged! (status: blocked)

@deepin-bot deepin-bot bot merged commit fd54ab0 into linuxdeepin:master Jan 22, 2026
15 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants